Skip to content

bugfix(gameengine): Fix logic in GameEngine::canUpdateRegularGameLogic()#2707

Open
xezon wants to merge 2 commits into
TheSuperHackers:mainfrom
xezon:xezon/fix-logic-update
Open

bugfix(gameengine): Fix logic in GameEngine::canUpdateRegularGameLogic()#2707
xezon wants to merge 2 commits into
TheSuperHackers:mainfrom
xezon:xezon/fix-logic-update

Conversation

@xezon
Copy link
Copy Markdown

@xezon xezon commented May 14, 2026 •
edited
Loading

This change fixes a logic error in GameEngine::canUpdateRegularGameLogic. It was spotted when loading a save game while max fps was uncapped, but after saveload it gets capped with TheGlobalData->m_useFpsLimit = true, which was then ignored in GameEngine::canUpdateRegularGameLogic. The result was an update timing discrepancy in GameEngine update, which caused glitches on game world visuals.

TODO

  • Replicate in Generals

@xezon xezon added this to the Decouple logic and rendering milestone May 14, 2026
@xezon xezon added Bug Something is not working right, typically is user facing Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour ThisProject The issue was introduced by this project, or this task is specific to this project labels May 14, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 14, 2026 •
edited
Loading

Greptile Summary

This PR fixes a logic bug in GameEngine::canUpdateRegularGameLogic() that caused visual glitches when loading a save game while FPS was uncapped and the post-load FPS cap (m_useFpsLimit = true) was subsequently ignored. The fix removes the premature !enabled short-circuit and delegates to getActualLogicTimeScaleFps/getActualFramesPerSecondLimit, which correctly account for the FPS-limit setting.

  • Replaces isLogicTimeScaleEnabled() / getLogicTimeScaleFps() / getFramesPerSecondLimit() with getActualLogicTimeScaleFps(flags) / getActualFramesPerSecondLimit() so the FPS cap state is always respected.
  • Adds a logicTimeQueryFlags parameter (passed as IgnoreFrozenTime | IgnoreHaltedGame) so the FPS comparison is evaluated independently of frozen/halted state, which is already handled at the call site.
  • Change is applied identically in both Generals/ and GeneralsMD/ trees.

Confidence Score: 5/5

Safe to merge — the change is a targeted, well-scoped fix for a reproducible visual glitch on save-game load.

The old !enabled short-circuit was the documented root cause, and the replacement delegates to getActualLogicTimeScaleFps/getActualFramesPerSecondLimit, both of which correctly incorporate the FPS-limit state. The flags passed at the call site match the existing downstream checks, so frozen/halted state is still honoured in update() as before.

No files require special attention.

Important Files Changed
Filename Overview
Generals/Code/GameEngine/Source/Common/GameEngine.cpp Core bug fix: removes the !enabled early-return, delegates FPS queries to getActualLogicTimeScaleFps/getActualFramesPerSecondLimit, and threads `IgnoreFrozenTime
Generals/Code/GameEngine/Include/Common/GameEngine.h Declaration updated to add UnsignedInt logicTimeQueryFlags parameter to canUpdateGameLogic and canUpdateRegularGameLogic; no issues found.
GeneralsMD/Code/GameEngine/Source/Common/GameEngine.cpp Identical fix mirrored for Zero Hour; change is consistent with the Generals counterpart.
GeneralsMD/Code/GameEngine/Include/Common/GameEngine.h Declaration updated identically to the Generals header; no issues found.

Reviews (2): Last reviewed commit: "Replicate in Generals" | Re-trigger Greptile

Mauller
Mauller approved these changes May 18, 2026
View reviewed changes
Copy link
Copy Markdown

@Mauller Mauller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

@xezon xezon force-pushed the xezon/fix-logic-update branch from adb29b1 to 03edd6c Compare May 20, 2026 19:51
@xezon
Copy link
Copy Markdown
Author

xezon commented May 20, 2026

Rebased.

Replicated in Generals without conflicts

D:\Projects\TheSuperHackers\GeneralsGameCode>FOR /F "delims=" %b IN ('git merge-base --fork-point main') DO git diff %b  1>changes.patch

D:\Projects\TheSuperHackers\GeneralsGameCode>git diff 842c7a53261c8e14772cd94ea678f6cc446637d8  1>changes.patch

D:\Projects\TheSuperHackers\GeneralsGameCode>git apply -p2 --directory=Generals --reject --whitespace=fix changes.patch
Checking patch Generals/Code/GameEngine/Include/Common/GameEngine.h...
Hunk #1 succeeded at 80 (offset 1 line).
Checking patch Generals/Code/GameEngine/Source/Common/GameEngine.cpp...
Hunk #1 succeeded at 649 (offset -164 lines).
Hunk #2 succeeded at 663 (offset -164 lines).
Hunk #3 succeeded at 684 (offset -164 lines).
Hunk #4 succeeded at 695 (offset -164 lines).
Hunk #5 succeeded at 745 (offset -164 lines).
Applied patch Generals/Code/GameEngine/Include/Common/GameEngine.h cleanly.
Applied patch Generals/Code/GameEngine/Source/Common/GameEngine.cpp cleanly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mauller Mauller Mauller approved these changes

Assignees

No one assigned

Labels

Bug Something is not working right, typically is user facing Gen Relates to Generals Minor Severity: Minor < Major < Critical < Blocker ThisProject The issue was introduced by this project, or this task is specific to this project ZH Relates to Zero Hour

Projects

None yet

Milestone

Decouple logic and rendering

Development

Successfully merging this pull request may close these issues.

2 participants

@xezon @Mauller